feat: access-api uses DID env variable when building its ucanto server id #275

gobengo · 2022-12-08T22:58:01Z

Motivation:

access service & client should use did:web:web3.storage #237

Notes

This is intended to be safe to merge/deploy as is. The new functionality can be opted-into via env vars
The ucanto server will only be constructed with an opts.id that is a signer withDID(...) iff process.env.DID is set to a DID
- If process.env.DID is unset, the ucanto server id signer will have a did() corresponding to config.PRIVATE_KEY (it will be a did:key of the corresponding public key)
Before this PR, many tests of the access-api worker were configured implicitly via dotenv with whatever the /.env.local file happened to contain. I added a worker test that asserts functionality that is dependent on the details of the env vars, so I made an affordance for tests to be able to configure the access-api worker with environment variables defined in the test case itself.

…er id

…o configureSigner

Gozala

Looks good to me! Thanks

Gozala · 2022-12-08T23:17:37Z

packages/access-api/src/config.js

+export function configureSigner(config) {
+  const signer = Signer.parse(config.PRIVATE_KEY)
+  if (config.DID) {
+    if (!isDID(config.DID)) {


I think it may be better to use existing functionality for this, for example this will parse and throw if it's invalid:

DID.match({ method: 'web' }).from(config.DID)

It will also take care of refusing did:key which would be invalid in this case.

…f new custom code

Gozala · 2022-12-09T00:41:41Z

packages/access-api/src/config.js

-  if (parts[0] !== 'did') return false
-  return true
+  try {
+    return Boolean(DID.match({}).from(object))


What’s the point of catching here to then throw ? You can simply just do .from instead of whole isDID thing.

Alternatively there’s also .is method which you could instead which will return boolean

👍 5184a55

hugomrdias · 2022-12-09T16:16:08Z

packages/access-api/src/utils/context.js

@@ -42,12 +41,12 @@ export function getContext(request, env, ctx) {
    env: config.ENV,
  })

-  const keypair = Signer.parse(config.PRIVATE_KEY)
+  const signer = configureSigner(config)


can you do this inside the loadConfig ?

👍 b9533c9

hugomrdias · 2022-12-09T16:18:31Z

packages/access-api/src/utils/context.js

  const url = new URL(request.url)
  const db = new D1QB(config.DB)
  return {
    log,
-    signer: keypair,
+    signer,


Suggested change

signer,

signer: Signer.parse(config.PRIVATE_KEY).withDID(config.ENV === 'production' ? 'did:web:ucan.web3.storage': `did:web:ucan-${config.ENV}.web3.storage`),

can't we just do this ? any error will be caught at dev time or CI.

I want the advertised DID to come from env variable. A '.web3.storage' domain won't be the best choice 100% of the times, e.g. on localhost or testing various scenarios. env variable gives me flexibility to get something working. once something works/tests end to end, can narrow down the configurability.

hugomrdias · 2022-12-09T16:22:59Z

packages/access-api/test/ucan.test.js

+      environment: {
+        ...process.env,
+        PRIVATE_KEY:
+          'MgCYWjE6vp0cn3amPan2xPO+f6EZ3I+KwuN1w2vx57vpJ9O0Bn4ci4jn8itwc121ujm7lDHkCW24LuKfZwIdmsifVysY=',


why do you need a new key here ?

I don't. Good catch. 🙏 d204331

…signer

…can.test

hugomrdias · 2022-12-09T19:09:53Z

packages/access-api/src/config.js

@@ -49,6 +55,9 @@ export function loadConfig(env) {
    // eslint-disable-next-line no-undef
    COMMITHASH: ACCOUNT_COMMITHASH,

+    DID,
+    signer,


can we get the did:key and did:web from the signer instance ?

console.log('signer did', signer.did().toString())

signer did did:web:exampe.com

console.log('signer archive', signer.toArchive())

signer archive {
id: 'did:web:exampe.com',
keys: {
'did:key:z6MkqBzPG7oNu7At8fktasQuS7QR7Tj7CujaijPMAgzdmAxD': Ed25519Signer(68) [Uint8Array] [
128, 38, 22, 140, 78, 175, 167, 71, 39, 221, 169,
143, 106, 125, 177, 60, 239, 159, 232, 70, 119, 35,
226, 176, 184, 221, 112, 218, 252, 121, 238, 250, 73,
244, 237, 1, 159, 135, 34, 226, 57, 252, 138, 220,
28, 215, 109, 110, 142, 110, 229, 12, 121, 2, 91,
110, 11, 184, 167, 217, 192, 135, 102, 178, 39, 213,
202, 198
]
}
}

hugomrdias · 2022-12-09T19:14:28Z

packages/access-api/src/config.js

  return {
    DEBUG: boolValue(vars.DEBUG),
    ENV: parseRuntimeEnv(vars.ENV),

-    PRIVATE_KEY: vars.PRIVATE_KEY,
+    PRIVATE_KEY,


we probably dont need this one anymore right ?

hugomrdias

LGTM

🤖 I have created a release *beep* *boop* --- ## [4.0.0](access-api-v3.0.0...access-api-v4.0.0) (2022-12-13) ### ⚠ BREAKING CHANGES * upgrade access-api @ucanto/* and @ipld/dag-ucan major versions ([#246](#246)) * access-client store decoupling ([#228](#228)) * upgrade to `@ucanto/{interface,principal}`@^4.0.0 ([#238](#238)) * follow up on the capabilities extract ([#239](#239)) * doc capabilities & make requierd nb non-optionals ([#159](#159)) * Remove 0.8 caps and add account delegation to the service ([#123](#123)) * bump to 0.9 ([#116](#116)) ### Features * [#153](#153) ([#177](#177)) ([d6d448c](d6d448c)) * access-api uses DID env variable when building its ucanto server id ([#275](#275)) ([311da78](311da78)) * **access-client:** cli and recover ([#207](#207)) ([adb3a8d](adb3a8d)) * account recover with email ([#149](#149)) ([6c659ba](6c659ba)) * bump to 0.9 ([#116](#116)) ([3e0b63f](3e0b63f)) * doc capabilities & make requierd nb non-optionals ([#159](#159)) ([6496773](6496773)) * follow up on the capabilities extract ([#239](#239)) ([ef5e779](ef5e779)) * Remove 0.8 caps and add account delegation to the service ([#123](#123)) ([878f8c9](878f8c9)), closes [#117](#117) [#121](#121) * Revert "feat!: upgrade to `@ucanto/{interface,principal}`@^4.0.0" ([#245](#245)) ([c182bbe](c182bbe)) * sync encode/decode delegations ([#276](#276)) ([ab981fb](ab981fb)) * upgrade access-api @ucanto/* and @ipld/dag-ucan major versions ([#246](#246)) ([5e663d1](5e663d1)) * upgrade to `@ucanto/{interface,principal}`@^4.0.0 ([#238](#238)) ([2f3bab8](2f3bab8)) ### Bug Fixes * 0.9 ([#78](#78)) ([1b1ed01](1b1ed01)) * add validation copy ([#257](#257)) ([7f50af4](7f50af4)), closes [#139](#139) * fix Access API cannot get space/info [#243](#243) ([#255](#255)) ([1bacd54](1bacd54)) * fix d1 migrations ([#264](#264)) ([fb8c09d](fb8c09d)) * make d1 spaces.metadata nullable and change to kysely ([#284](#284)) ([c8a9ce5](c8a9ce5)), closes [#280](#280) * make multiformats 9 go away ([#133](#133)) ([2668880](2668880)) * miniflare dev script ([#137](#137)) ([1c5a4e2](1c5a4e2)) * testing staging deploy ([3d500fa](3d500fa)) * try to deploy api staging ([18b7b29](18b7b29)) * try to deploy staging ([c616818](c616818)) ### Code Refactoring * access-client store decoupling ([#228](#228)) ([a785278](a785278)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

…r id (#275) Motivation: * #237 Notes * This is intended to be safe to merge/deploy as is. The new functionality can be opted-into via env vars * The ucanto server will only be constructed with an opts.id that is a signer `withDID(...)` iff `process.env.DID` is set to a DID * If `process.env.DID` is unset, the ucanto server id signer will have a `did()` corresponding to `config.PRIVATE_KEY` (it will be a `did:key` of the corresponding public key) * Before this PR, many tests of the access-api worker were configured implicitly via dotenv with whatever the `/.env.local` file happened to contain. I added a worker test that asserts functionality that is dependent on the details of the env vars, so I made an affordance for [tests to be able to configure the access-api worker with environment variables defined in the test case itself](https://github.com/web3-storage/w3protocol/pull/275/files#diff-e20ffc550d006747790e7b67da280446c1cd1d2d4f72ccb5df04f62cbb6423daR149).

🤖 I have created a release *beep* *boop* --- ## [4.0.0](access-api-v3.0.0...access-api-v4.0.0) (2022-12-13) ### ⚠ BREAKING CHANGES * upgrade access-api @ucanto/* and @ipld/dag-ucan major versions ([#246](#246)) * access-client store decoupling ([#228](#228)) * upgrade to `@ucanto/{interface,principal}`@^4.0.0 ([#238](#238)) * follow up on the capabilities extract ([#239](#239)) * doc capabilities & make requierd nb non-optionals ([#159](#159)) * Remove 0.8 caps and add account delegation to the service ([#123](#123)) * bump to 0.9 ([#116](#116)) ### Features * [#153](#153) ([#177](#177)) ([7fbf2a1](7fbf2a1)) * access-api uses DID env variable when building its ucanto server id ([#275](#275)) ([e8326ec](e8326ec)) * **access-client:** cli and recover ([#207](#207)) ([720dafb](720dafb)) * account recover with email ([#149](#149)) ([91ad47d](91ad47d)) * bump to 0.9 ([#116](#116)) ([29cf63c](29cf63c)) * doc capabilities & make requierd nb non-optionals ([#159](#159)) ([f6b6d06](f6b6d06)) * follow up on the capabilities extract ([#239](#239)) ([717fcaa](717fcaa)) * Remove 0.8 caps and add account delegation to the service ([#123](#123)) ([c3c58b9](c3c58b9)), closes [#117](#117) [#121](#121) * Revert "feat!: upgrade to `@ucanto/{interface,principal}`@^4.0.0" ([#245](#245)) ([197439e](197439e)) * sync encode/decode delegations ([#276](#276)) ([9d48372](9d48372)) * upgrade access-api @ucanto/* and @ipld/dag-ucan major versions ([#246](#246)) ([65d191c](65d191c)) * upgrade to `@ucanto/{interface,principal}`@^4.0.0 ([#238](#238)) ([309aff0](309aff0)) ### Bug Fixes * 0.9 ([#78](#78)) ([561c68b](561c68b)) * add validation copy ([#257](#257)) ([0bd49ce](0bd49ce)), closes [#139](#139) * fix Access API cannot get space/info [#243](#243) ([#255](#255)) ([1a74031](1a74031)) * fix d1 migrations ([#264](#264)) ([5b8a6e7](5b8a6e7)) * make d1 spaces.metadata nullable and change to kysely ([#284](#284)) ([7f09479](7f09479)), closes [#280](#280) * make multiformats 9 go away ([#133](#133)) ([cdb4109](cdb4109)) * miniflare dev script ([#137](#137)) ([f2cffb2](f2cffb2)) * testing staging deploy ([975cffc](975cffc)) * try to deploy api staging ([5130464](5130464)) * try to deploy staging ([4e56657](4e56657)) ### Code Refactoring * access-client store decoupling ([#228](#228)) ([64c7496](64c7496)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

access-api uses DID env variable when building its ucanto server sign…

6504306

…er id

gobengo temporarily deployed to dev December 8, 2022 22:58 — with GitHub Actions Inactive

access-api add test ensuring error when non-did config.DID provided t…

a4785dd

…o configureSigner

gobengo changed the title ~~access-api uses DID env variable when building its ucanto server id~~ feat: access-api uses DID env variable when building its ucanto server id Dec 8, 2022

gobengo temporarily deployed to dev December 8, 2022 23:06 — with GitHub Actions Inactive

gobengo requested review from Gozala and hugomrdias December 8, 2022 23:14

Gozala approved these changes Dec 8, 2022

View reviewed changes

gobengo added 2 commits December 8, 2022 15:49

access-api update isDID type guard to use @ucanto/validator instead o…

91edd06

…f new custom code

pnpm-lock

795582a

gobengo temporarily deployed to dev December 8, 2022 23:51 — with GitHub Actions Inactive

Gozala reviewed Dec 9, 2022

View reviewed changes

hugomrdias suggested changes Dec 9, 2022

View reviewed changes

access-api configureSigner happens inside loadConfig and sets config.…

b9533c9

…signer

gobengo temporarily deployed to dev December 9, 2022 17:40 — with GitHub Actions Inactive

remove unnecessary PRIVATE_KEY environment var from access-api/test/u…

d204331

…can.test

gobengo temporarily deployed to dev December 9, 2022 17:45 — with GitHub Actions Inactive

access-api/src/config remove isDID function

5184a55

gobengo temporarily deployed to dev December 9, 2022 17:48 — with GitHub Actions Inactive

gobengo requested a review from hugomrdias December 9, 2022 17:49

gobengo mentioned this pull request Dec 9, 2022

access service & client should use did:web:web3.storage #237

Closed

5 tasks

hugomrdias suggested changes Dec 9, 2022

View reviewed changes

gobengo added 2 commits December 9, 2022 15:17

improve configureSigner test

a629263

config loadConfig no longer has DID or PRIVATE_KEY on it

852951f

gobengo requested a review from hugomrdias December 9, 2022 23:19

gobengo temporarily deployed to dev December 9, 2022 23:19 — with GitHub Actions Inactive

hugomrdias approved these changes Dec 12, 2022

View reviewed changes

gobengo merged commit 311da78 into main Dec 12, 2022

gobengo deleted the issues/237-access-api-didweb branch December 12, 2022 18:21

gobengo self-assigned this Dec 12, 2022

hugomrdias mentioned this pull request Dec 12, 2022

chore(main): release access-api 4.0.0 #113

Merged

gobengo mentioned this pull request Dec 12, 2022

Use same did across access / upload API services #183

Closed

it-dag-house mentioned this pull request Mar 23, 2023

chore(main): release access-api 11.0.0-rc.0 #620

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: access-api uses DID env variable when building its ucanto server id #275

feat: access-api uses DID env variable when building its ucanto server id #275

gobengo commented Dec 8, 2022 •

edited

Loading

Gozala left a comment

Gozala Dec 8, 2022

gobengo Dec 9, 2022

Gozala Dec 9, 2022

gobengo Dec 9, 2022

hugomrdias Dec 9, 2022

gobengo Dec 9, 2022

hugomrdias Dec 9, 2022

gobengo Dec 9, 2022 •

edited

Loading

hugomrdias Dec 9, 2022

gobengo Dec 9, 2022

hugomrdias Dec 9, 2022

gobengo Dec 9, 2022

hugomrdias Dec 9, 2022

gobengo Dec 9, 2022

hugomrdias left a comment

	signer,
	signer: Signer.parse(config.PRIVATE_KEY).withDID(config.ENV === 'production' ? 'did:web:ucan.web3.storage': `did:web:ucan-${config.ENV}.web3.storage`),

feat: access-api uses DID env variable when building its ucanto server id #275

feat: access-api uses DID env variable when building its ucanto server id #275

Conversation

gobengo commented Dec 8, 2022 • edited Loading

Gozala left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gobengo Dec 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hugomrdias left a comment

Choose a reason for hiding this comment

gobengo commented Dec 8, 2022 •

edited

Loading

gobengo Dec 9, 2022 •

edited

Loading